Skip to content

Conversation

ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Sep 25, 2025

This PR proposes using wrappers for fftfreq, fftshift, ifftshift, and rfftfreq instead of importing them from scipy.fft or numpy.fft directly

These thin wrappers can avoid infinite recursion of dependencies, i.e., when importing Intel NumPy with Intel SciPy in the environment

…importing directly

avoids circular dependencies under certain conditions
Copy link
Collaborator

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix LGTM! But need to add the PR to the changelog also.

@ndgrigorian
Copy link
Collaborator Author

The fix LGTM! But need to add the PR to the changelog also.

done

@ekomarova
Copy link
Collaborator

Will a change like this always require scipy as a mkl_fft dependency, or can it still be optional?

@ndgrigorian
Copy link
Collaborator Author

ndgrigorian commented Sep 26, 2025

Will a change like this always require scipy as a mkl_fft dependency, or can it still be optional?

It can still be optional—since the scipy interface is only exposed in interfaces/__init__.py if SciPy can be imported, this doesn't change anything (scipy_fft.py already needed to import it before)

alternatively, the import could be made lazy, so we could do from scipy.fft import fftshift in the def of fftshift.

@ndgrigorian ndgrigorian merged commit 3cb11f3 into master Sep 26, 2025
85 checks passed
@ndgrigorian ndgrigorian deleted the complete-scipy-interface-namespace branch September 26, 2025 18:13
@ndgrigorian ndgrigorian changed the title Use wrappers to complete SciPy interface namespace Use wrappers to complete SciPy and NumPy interface namespaces Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants